Skip to content

Fix ordering of time-varying statespace matices for UnivariateFilter's kalman step#653

Merged
jessegrabowski merged 2 commits intopymc-devs:mainfrom
sebcroft:fix-shape-bug
Apr 25, 2026
Merged

Fix ordering of time-varying statespace matices for UnivariateFilter's kalman step#653
jessegrabowski merged 2 commits intopymc-devs:mainfrom
sebcroft:fix-shape-bug

Conversation

@sebcroft
Copy link
Copy Markdown
Contributor

This aims to address #612

Following the error in #612, the design matrix Z should have shape (2,3) not (3,3). Intial thought is the latter shape corresponds to the transition matrix T.

In BaseFilter, the kalman_step function calls the method unpack_args to reorganise the inputs into a standardized order. This is needed because when the user specifies time varying matrices, the kalman_step arguments (y, a, P, c, d, T, Z, R, H, Q) can get 'mixed-up' depending on which variables are assigned to the sequences and non_sequences in pytensor.scan.

UnivariateFilter redefines the kalman_step function and does not currently call unpack_args. Therefore, when the selection matrix R is the only time varying matrix (as in #612) it gets sent to the sequences/start (with y) and shifts the remaining args up by 1. As a result, T is incorrectly used for the Z input, which aligns with the shape error.

When all matrices are time-invariant unpack_args is not required because the correct ordering is preserved -- this is why UnivariateFilter works for the time-invariant matrices as mentioned in #612.

To address this I have added the method unpack_args to the kalman_step in UnivariateFilter.

Some extra stuff:

The above changes should hopefully be enough, but to keep things consistent with the BaseFilter you could possibly deal with the nan values using handle_missing_values instead i.e.:

def kalman_step(self, *args):
    
    y, a, P, c, d, T, Z, R, H, Q = self.unpack_args(args)
    
    nan_mask = pt.isnan(y)
    
    y_masked, Z_masked, H_masked, all_nan_flag = self.handle_missing_values(y, Z, H)
    
    result = pytensor.scan(
        self._univariate_inner_filter_step,
        sequences=[y_masked, Z_masked, d, pt.diag(H_masked), nan_mask],
        outputs_info=[a, P, None, None, None],
        name="univariate_inner_scan",
        return_updates=False,
    )

I'm happy to add this if you think it's best!

@jessegrabowski
Copy link
Copy Markdown
Member

Great catch -- thanks for this!

I think it makes sense to also add the missing value handling if you don't object.

Also make sure you install and run pre-commit in your local dev environment. It's just pip install pre-commit; pre-commit install; pre-commit run --all (after you do install it will automatically run each time you git commit)

@jessegrabowski jessegrabowski added bug Something isn't working enhancements New feature or request statespace labels Feb 23, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.21%. Comparing base (71626a9) to head (f73b15c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #653       +/-   ##
===========================================
- Coverage   80.19%   24.21%   -55.98%     
===========================================
  Files          73       73               
  Lines        8088     8086        -2     
===========================================
- Hits         6486     1958     -4528     
- Misses       1602     6128     +4526     
Files with missing lines Coverage Δ
pymc_extras/statespace/filters/kalman_filter.py 100.00% <100.00%> (ø)

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jessegrabowski jessegrabowski merged commit 87f41c4 into pymc-devs:main Apr 25, 2026
29 checks passed
@jessegrabowski
Copy link
Copy Markdown
Member

Thanks @sebcroft ! Sorry it took a minute to get in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancements New feature or request statespace

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants